Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extra null check on diff #12

Closed
wants to merge 14 commits into from
Closed

extra null check on diff #12

wants to merge 14 commits into from

Conversation

cristianvogel
Copy link

When building up more in the Elem dsp graph, the plugin could fail to register new refs related to any extra dsp blocks in the render chain.

This additional null check seems to make things flow easier.

@nick-thompson
Copy link
Contributor

Hey @cristianvogel I'm sorry for the delay! I totally missed the notification on this PR.

Thanks for taking the time to open this. I'm not sure this is the right change– if we're comparing the new state to the old state and find that the new state is null, that must mean that the state object dispatched by the native side is empty or failed to serialize or something. That part is ok, but if that's true, then we should neither render the graph nor update any refs because we have no state to base our graph rendering on. So I think the spirit of this change is a good one, but it should be made inside the globalThis.__receiveStateChange__ hook: right after parsing the incoming state, if it's null, early return. How does that sound?

@cristianvogel cristianvogel closed this by deleting the head repository Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants